-
Notifications
You must be signed in to change notification settings - Fork 2.8k
BROS-114: Global Custom Hotkeys #7784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
…ation Signed-off-by: Michael Malyuk <michael.malyuk@icloud.com>
Signed-off-by: Michael Malyuk <michael.malyuk@icloud.com>
Signed-off-by: Michael Malyuk <michael.malyuk@icloud.com>
Signed-off-by: Michael Malyuk <michael.malyuk@icloud.com>
Signed-off-by: Michael Malyuk <michael.malyuk@icloud.com>
✅ Deploy Preview for heartex-docs canceled.
|
✅ Deploy Preview for label-studio-docs-new-theme canceled.
|
✅ Deploy Preview for label-studio-playground canceled.
|
✅ Deploy Preview for label-studio-storybook canceled.
|
@@ -90,7 +90,8 @@ class User(UserMixin, AbstractBaseUser, PermissionsMixin, UserLastActivityMixin) | |||
last_name = models.CharField(_('last name'), max_length=256, blank=True) | |||
phone = models.CharField(_('phone'), max_length=256, blank=True) | |||
avatar = models.ImageField(upload_to=hash_upload, blank=True) | |||
|
|||
custom_hotkeys = models.JSONField(default=dict, blank=True, null=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
custom_hotkeys = models.JSONField(default=dict, blank=True, null=True) | |
custom_hotkeys = models.JSONField( | |
_('custom hotkeys'), | |
default=dict, | |
blank=True, | |
null=True, | |
help_text=_('Custom keyboard shortcuts configuration for the user interface') | |
) |
}; | ||
|
||
// Handle platform translation toggle | ||
const handleTogglePlatformTranslation = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems never used - remove it
const api = useAPI(); | ||
|
||
// Function to identify which hotkeys were modified from defaults | ||
const getModifiedHotkeys = (currentHotkeys) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const getModifiedHotkeys = useMemo(() => {
...
}, [hotkeys]);
@@ -0,0 +1,599 @@ | |||
import { useEffect, useState } from "react"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import { useEffect, useState } from "react"; | |
import { useEffect, useState, useMemo, useReact } from "react"; |
this file misses memoization pattern, which results in the degraded performance and uneccessary rerenders. I'd suggest wrapping places with useMemo
and useCallback
where it is necessary for:
- prevent recomputations of potentially expensive operations (e.g.
getModifiedHotkeys
) - prevent components rerenders due to non-memoized handlers (e.g. handlers in
<HotkeySection>
)
}; | ||
|
||
// Save hotkeys to API function | ||
const saveHotkeysToAPI = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const saveHotkeysToAPI = async () => { | |
const saveHotkeysToAPI = useCallback(async () => { |
}, []); | ||
|
||
// Handle enabling/disabling all hotkeys | ||
const handleToggleAllHotkeys = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it is not used - remove it
@@ -98,26 +98,86 @@ | |||
</template> | |||
|
|||
<script id="app-settings" nonce="{{request.csp_nonce}}"> | |||
|
|||
var __customHotkeys = {{ user.custom_hotkeys|json_dumps_ensure_ascii|safe }}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not creating mergedEditorKeymap
in Django code instead of custom inline js ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Django doesn't know much about the defaults for the keys, and we don't want to have two sets of defaults, one on the backend, and another on the frontend, all of the definitions and processing is done at the frontend level, this follows DRY principle.
@@ -0,0 +1,88 @@ | |||
|
|||
import json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test suite should use pytest
and follow the principles described in https://github.com/HumanSignal/label-studio-enterprise/blob/develop/.cursor/rules/backend-unit-tests.mdc
Example:
import json
import pytest
from users.serializers import HotkeysSerializer # Import from your actual app
class TestHotkeysSerializer:
"""Tests for the HotkeysSerializer following pytest guidelines"""
@pytest.fixture
def valid_hotkeys(self):
"""Fixture providing valid hotkeys data for testing"""
return {
"editor:save": {"key": "ctrl+s", "active": True},
"editor:open": {"key": "ctrl+o", "active": False},
"editor:cut": {"key": "ctrl+x"},
"navigation:home": {"key": "alt+h", "active": True}
}
@pytest.fixture
def valid_serializer_data(self, valid_hotkeys):
"""Fixture providing valid serializer data"""
return {"custom_hotkeys": valid_hotkeys}
def test_valid_data(self, valid_serializer_data):
"""Test serializer validation with valid hotkeys data.
This test validates:
- Serializer accepts properly formatted hotkeys dictionary
- All required fields are present and valid
- Both active and inactive hotkeys are handled correctly
- Optional 'active' field defaults appropriately
Critical validation: Ensures well-formed hotkey configurations
are accepted without errors for user customization features.
"""
serializer = HotkeysSerializer(data=valid_serializer_data)
assert serializer.is_valid()
def test_invalid_format_not_dict(self):
"""Test serializer rejects non-dictionary custom_hotkeys.
This test validates:
- Serializer properly validates data type of custom_hotkeys field
- Non-dictionary values (lists, strings, etc.) are rejected
- Appropriate validation errors are returned
Critical validation: Prevents malformed data from corrupting
user hotkey preferences and ensures data integrity.
"""
data = {"custom_hotkeys": ["not a dictionary"]}
serializer = HotkeysSerializer(data=data)
assert not serializer.is_valid()
assert 'custom_hotkeys' in serializer.errors
def test_invalid_action_key_format(self):
"""Test serializer rejects action keys without proper format.
This test validates:
- Action keys must follow 'category:action' format with colon separator
- Malformed action keys (missing colon) are rejected
- Validation error is properly raised and contains expected field
Critical validation: Ensures hotkey action identifiers follow
expected naming convention for proper system integration.
"""
# Missing colon
invalid_data = {
"custom_hotkeys": {
"editorsave": {"key": "ctrl+s"}
}
}
serializer = HotkeysSerializer(data=invalid_data)
assert not serializer.is_valid()
assert 'custom_hotkeys' in serializer.errors
def test_empty_action_key(self):
"""Test serializer rejects empty action keys.
This test validates:
- Empty string action keys are not allowed
- Validation properly catches and rejects empty identifiers
- Error response includes the problematic field
Critical validation: Prevents creation of hotkeys with
undefined action identifiers that would break functionality.
"""
invalid_data = {
"custom_hotkeys": {
"": {"key": "ctrl+s"}
}
}
serializer = HotkeysSerializer(data=invalid_data)
assert not serializer.is_valid()
assert 'custom_hotkeys' in serializer.errors
def test_missing_key_in_hotkey_data(self):
"""Test serializer rejects hotkey data without required 'key' field.
This test validates:
- The 'key' field is mandatory for each hotkey configuration
- Hotkey objects missing the 'key' field are rejected
- Validation error properly identifies the missing required field
Critical validation: Ensures all hotkeys have keyboard combinations
defined, preventing non-functional hotkey configurations.
"""
invalid_data = {
"custom_hotkeys": {
"editor:save": {"active": True} # Missing 'key'
}
}
serializer = HotkeysSerializer(data=invalid_data)
assert not serializer.is_valid()
assert 'custom_hotkeys' in serializer.errors
def test_invalid_key_value(self):
"""Test serializer rejects invalid key values.
This test validates:
- Empty string key values are not allowed
- Key field must contain actual keyboard combination
- Validation catches and rejects meaningless key definitions
Critical validation: Ensures hotkeys have valid keyboard
combinations that can be processed by the frontend system.
"""
invalid_data = {
"custom_hotkeys": {
"editor:save": {"key": ""} # Empty key
}
}
serializer = HotkeysSerializer(data=invalid_data)
assert not serializer.is_valid()
assert 'custom_hotkeys' in serializer.errors
def test_invalid_active_flag(self):
"""Test serializer rejects non-boolean active flags.
This test validates:
- The 'active' field must be a boolean value when provided
- String representations like 'yes'/'no' are not accepted
- Type validation is properly enforced for optional fields
Critical validation: Ensures hotkey enable/disable state
is properly typed for consistent system behavior.
"""
invalid_data = {
"custom_hotkeys": {
"editor:save": {"key": "ctrl+s", "active": "yes"} # Should be boolean
}
}
serializer = HotkeysSerializer(data=invalid_data)
assert not serializer.is_valid()
assert 'custom_hotkeys' in serializer.errors
@@ -0,0 +1,263 @@ | |||
// Section styling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to use utility classes and semantic tokens over the custom CSS, otherwise it won't be compatible with the current design system (and for example, it will break compatibility with Dark mode switch)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example of rewrite:
// Section styling
.sectionHeader {
@apply mb-wide flex items-center;
}
.sectionTitle {
@apply m-0 text-headline-small font-semibold;
}
.globalControls {
@apply flex gap-base;
}
// Loading state
.loadingContainer {
@apply flex flex-col items-center justify-center p-1200 text-neutral-content-subtle;
}
.loadingSpinner {
@apply w-1000 h-1000 border-300 rounded-full border-neutral-border-subtle border-t-primary-border animate-spin mb-base;
}
// Hotkey sections
.hotkeySections {
@apply flex flex-col gap-800;
}
.hotkeySection {
@apply bg-neutral-background rounded-small border border-neutral-border overflow-hidden;
}
.hotkeySectionHeader {
@apply flex justify-between items-center p-base px-wide bg-neutral-surface border-b border-neutral-border;
}
.hotkeySectionTitle {
@apply m-0 mb-100 text-title-large font-semibold text-neutral-content;
}
.hotkeySectionDescription {
@apply m-0 text-body-small text-neutral-content-subtle;
}
.hotkeyList {
@apply p-base px-wide;
}
.emptyHotkeyList {
@apply py-800 text-center text-neutral-content-subtle italic;
}
// Hotkey item styling
.hotkeyItem {
@apply flex justify-between items-center py-300 border-b border-neutral-border-subtler;
&:last-child {
@apply border-b-0;
}
}
.hotkeyItemDisabled {
@apply opacity-60;
}
.hotkeyDetails {
@apply flex-1 mr-base;
}
.hotkeyLabel {
@apply font-medium mb-100;
}
.hotkeyDescription {
@apply text-body-small text-neutral-content-subtle;
}
.hotkeyControls {
@apply flex items-center gap-300;
}
.hotkeyKey {
@apply flex items-center cursor-pointer;
&:hover {
.keyboardKey {
@apply bg-primary-emphasis border-primary-border;
}
}
}
.hotkeyRemove {
@apply flex items-center justify-center w-600 h-600 rounded-full border-0 bg-negative-emphasis text-negative-content text-title-medium leading-none cursor-pointer transition-all duration-200;
&:hover {
@apply bg-negative-surface text-neutral-background;
}
}
// Editing mode styling
.hotkeyItemEditing {
@apply flex flex-col py-300 border-b border-neutral-border-subtler;
&:last-child {
@apply border-b-0;
}
.hotkeyLabel {
@apply mb-300;
}
}
.hotkeyEditControls {
@apply flex gap-base;
}
.keyRecordingBox {
@apply flex-1 flex items-center justify-center min-h-1000 py-200 px-base border border-neutral-border rounded-smaller bg-neutral-background cursor-pointer transition-all duration-200;
&:hover {
@apply border-primary-border;
}
&:focus {
@apply outline-none border-primary-border shadow-[0_0_0_2px] shadow-primary-focus-outline;
}
}
.recording {
@apply bg-primary-emphasis border-primary-border shadow-[0_0_0_2px] shadow-primary-focus-outline;
.recordingPrompt {
@apply text-primary-content font-medium animate-pulse;
}
}
.placeholderText {
@apply text-neutral-content-subtle;
}
.editActions {
@apply flex flex-col gap-200;
}
// Keyboard key styling
.keyboardKey {
@apply inline-flex items-center justify-center min-w-700 h-700 px-150 rounded-smaller bg-neutral-surface border border-neutral-border shadow-[0_2px_0] shadow-neutral-border text-body-smaller font-semibold uppercase text-neutral-content transition-all duration-200;
}
.keySeparator {
@apply mx-100 text-neutral-content-subtle;
}
.keyCombo {
@apply flex items-center gap-100;
}
@@ -142,7 +142,7 @@ def user_login(request): | |||
|
|||
|
|||
@login_required | |||
def user_account(request): | |||
def user_account(request, sub_path=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like a hack - provide at least a comment that it will be used as a redirect to a specific section like hotkeys
Co-authored-by: niklub <nikolai@heartex.com>
Co-authored-by: niklub <nikolai@heartex.com>
Reason for Change
Problem:
Label Studio has a rich set of keyboard shortcuts, but users previously had no way to customize them. This limited accessibility and forced users into rigid default behaviors.
Solution:
This PR introduces a fully-featured, user-level hotkey customization system. Users can override default shortcuts by section (e.g. annotation, regions, video), manage them in a dedicated Hotkeys tab in Account Settings, and import/export configurations for reuse or team sharing.
Rollout Strategy
Testing
HotkeysSerializer
validation logicPOST /api/current-user/hotkeys
custom_hotkeys
field toUser
modelRisks
Reviewer Notes
users/serializers.py
– Hotkey schema validationHotkey.ts
– Lookup/merge logicAccountSettings/sections/Hotkeys
– Full UI implementationlookupHotkey('section:action')
across the app for dynamic resolutiondefaults.js
General Notes
This upgrade significantly improves usability and flexibility, especially for power users. It also sets the stage for future team-wide profiles or shareable hotkey presets.